feat: add Docker support and comprehensive test coverage#12
Conversation
Introduce Docker setup with a Dockerfile for building the Grimoire API service and a docker-compose.yml for local development. This setup improves the development environment's consistency and ease of use. Update project files and dependencies to .NET 10, ensuring latest features and performance enhancements. Added .dockerignore to optimize Docker builds by excluding unneeded files. Enhance testing coverage with the addition of end-to-end (E2E), integration, and unit tests using xUnit. These tests verify application functionality, reducing bugs and ensuring robust code changes. Added infrastructure for Testcontainers for isolated DB testing environments. Modified SecretRepository to handle EF Core 10's limitations with nullable timestamp comparisons in SQL, improving query reliability for determining active secret versions.
Reviewer's GuideAdds Docker build/runtime support, health checks, and extensive automated coverage (unit, integration, and E2E) while slightly refactoring secret version retrieval for EF Core 10 compatibility. Sequence diagram for SecretRepository.GetActiveVersionAsync two-stage filteringsequenceDiagram
participant caller as Caller
participant repo as SecretRepository
participant dbctx as GrimoireDbContext
participant db as Database
caller->>repo: GetActiveVersionAsync(secretId, environmentId, now, ct)
repo->>dbctx: SecretVersions.Where(SecretId, EnvironmentId, IsEnabled)
dbctx->>db: Execute SQL on indexable columns
db-->>dbctx: Candidate rows ordered by Version desc
dbctx-->>repo: List of SecretVersion candidates
repo->>repo: In-memory filter on NotBefore and ExpiresAt vs now
repo-->>caller: SecretVersion activeVersion or null
Class diagram for SecretRepository and new test infrastructureclassDiagram
class SecretRepository {
+Task~List~SecretVersion~~ GetVersionsAsync(Guid secretId, Guid environmentId, CancellationToken ct)
+Task~SecretVersion?~ GetActiveVersionAsync(Guid secretId, Guid environmentId, DateTimeOffset now, CancellationToken ct)
+Task~int~ GetNextVersionNumberAsync(Guid secretId, Guid environmentId, CancellationToken ct)
}
class Program {
<<partial>>
}
class WebApplicationFactoryOfProgram {
<<external>>
}
class GrimoireWebApplicationFactory {
+GrimoireWebApplicationFactory()
+ConfigureWebHost(IWebHostBuilder builder)
}
class GrimoireApiFixture {
+Task InitializeAsync()
+Task DisposeAsync()
+HttpClient CreateClient()
}
class IntegrationTestsAssembly {
<<tests>>
}
class E2eTestsAssembly {
<<tests>>
}
GrimoireWebApplicationFactory --|> WebApplicationFactoryOfProgram
IntegrationTestsAssembly --> GrimoireWebApplicationFactory : uses
E2eTestsAssembly --> GrimoireApiFixture : uses
GrimoireApiFixture --> Program : starts Dockerized API
SecretRepository --> Program : used within API dependency injection
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. β How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. π¦ How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. βΉοΈ Review infoβοΈ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: π Files selected for processing (28)
β¨ Finishing Touchesπ§ͺ Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
# Conflicts: # src/Grimoire.Api/Grimoire.Api.csproj # src/Grimoire.Consumer/Grimoire.Consumer.csproj # src/Grimoire.Core/Grimoire.Core.csproj # src/Grimoire.Infrastructure/Grimoire.Infrastructure.csproj # src/Grimoire.Infrastructure/Persistence/Repositories/SecretRepository.cs # tests/Grimoire.Tests/Grimoire.Tests.csproj
|
Failed to generate code suggestions for PR |
π₯ Formatting issues detected
β‘ Please run |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Caution Review the following alerts detected in dependencies. According to your organization's Security Policy, you must resolve all "Block" alerts before proceeding. Learn more about Socket for GitHub.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- In SecretRepository.GetActiveVersionAsync, you now materialize all candidate versions into a list before applying the date filters; consider switching to AsEnumerable() and applying the date-based FirstOrDefault on the resulting sequence to avoid allocating a full list and to keep the logic streaming.
- In the Dockerfile, the ENV line sets ConnectionStrings__Default with surrounding quotes ("Data Source=/data/grimoire.db"), which Docker treats as part of the value; you may want to remove the quotes so the connection string is parsed correctly by .NET.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In SecretRepository.GetActiveVersionAsync, you now materialize all candidate versions into a list before applying the date filters; consider switching to AsEnumerable() and applying the date-based FirstOrDefault on the resulting sequence to avoid allocating a full list and to keep the logic streaming.
- In the Dockerfile, the ENV line sets ConnectionStrings__Default with surrounding quotes ("Data Source=/data/grimoire.db"), which Docker treats as part of the value; you may want to remove the quotes so the connection string is parsed correctly by .NET.
## Individual Comments
### Comment 1
<location path="Dockerfile" line_range="16" />
<code_context>
+FROM mcr.microsoft.com/dotnet/aspnet:10.0 AS final
+WORKDIR /app
+
+RUN mkdir -p /data && chmod 777 /data
+
+EXPOSE 8080
</code_context>
<issue_to_address>
**π¨ suggestion (security):** Relaxed `chmod 777` on `/data` is overly permissive and could be tightened.
World-writable permissions on `/data` are broader than necessary and weaken the containerβs security posture. Where possible, run the app as a non-root user and set ownership and permissions for that user only (e.g., `chown` to a dedicated UID/GID and use something like `chmod 750`). This keeps the volume writable for the app without granting global write access.
Suggested implementation:
```
FROM mcr.microsoft.com/dotnet/aspnet:10.0 AS final
WORKDIR /app
RUN groupadd -r app && useradd -r -g app app && \
mkdir -p /data && chown app:app /data && chmod 750 /data
```
```
COPY --from=build /app/publish .
USER app
ENTRYPOINT ["dotnet", "Grimoire.Api.dll"]
```
</issue_to_address>
### Comment 2
<location path="docker-compose.yml" line_range="16-17" />
<code_context>
+ Cors__AllowedOrigins__0: "http://localhost:5173"
+ volumes:
+ - grimoire-data:/data
+ healthcheck:
+ test: ["CMD", "curl", "-f", "http://localhost:8080/health"]
+ interval: 10s
+ timeout: 5s
</code_context>
<issue_to_address>
**issue (bug_risk):** Healthcheck relies on `curl`, which may not be present in the runtime image.
The `mcr.microsoft.com/dotnet/aspnet` base image does not include `curl` by default, so this healthcheck will fail with `command not found` unless you install it. Either add `curl` to the final image or use a mechanism thatβs already available in the container (e.g., `wget` if present, or a simple `/dev/tcp` shell check).
</issue_to_address>
### Comment 3
<location path="tests/Grimoire.IntegrationTests/Management/ApplicationsTests.cs" line_range="12-19" />
<code_context>
+ private HttpClient Client => factory.CreateManagementClient();
+
+ [Fact]
+ public async Task GetApplications_ReturnsEmptyListInitially()
+ {
+ var response = await Client.GetAsync("/api/management/applications");
+ Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+ var json = JsonNode.Parse(await response.Content.ReadAsStringAsync())!.AsArray();
+ Assert.NotNull(json);
+ }
+
</code_context>
<issue_to_address>
**suggestion (testing):** Strengthen assertion to actually enforce an empty list as the method name describes.
This test name implies the response body should be an empty collection, but it only asserts `json` is non-null. To properly verify the initial state and prevent regressions, also assert that the array is empty, e.g. `Assert.Empty(json);` or `Assert.Equal(0, json.Count);`.
```suggestion
[Fact]
public async Task GetApplications_ReturnsEmptyListInitially()
{
var response = await Client.GetAsync("/api/management/applications");
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
var json = JsonNode.Parse(await response.Content.ReadAsStringAsync())!.AsArray();
Assert.NotNull(json);
Assert.Empty(json);
}
```
</issue_to_address>
### Comment 4
<location path="tests/Grimoire.E2eTests/E2eTests.cs" line_range="94-98" />
<code_context>
+ }
+
+ [Fact]
+ public async Task RotateKey_OldKeyInvalid_NewKeyWorks()
+ {
+ var mgmt = fixture.CreateManagementClient();
+ var (slug, oldKey) = await CreateApplicationAsync(mgmt);
+
+ var rotateResp = await mgmt.PostAsync($"/api/management/applications/{slug}/rotate-key", null);
+ var rotateJson = JsonNode.Parse(await rotateResp.Content.ReadAsStringAsync())!;
+ var newKey = rotateJson["plainApiKey"]!.GetValue<string>();
+
+ // Old key should no longer work
</code_context>
<issue_to_address>
**suggestion (testing):** Assert the rotate-key response status code before parsing the body to avoid brittle failures.
In this E2E test, `rotateResp.Content` is parsed before confirming the response succeeded. If the endpoint returns a non-200 status (e.g. 500, 401), the test will fail with a JSON parsing error instead of a clear assertion. Add an explicit status assertion (e.g. `Assert.Equal(HttpStatusCode.OK, rotateResp.StatusCode);` or `rotateResp.EnsureSuccessStatusCode();`) before parsing `rotateJson` to keep the testβs intent and failure mode clear.
```suggestion
var (slug, oldKey) = await CreateApplicationAsync(mgmt);
var rotateResp = await mgmt.PostAsync($"/api/management/applications/{slug}/rotate-key", null);
rotateResp.EnsureSuccessStatusCode();
var rotateJson = JsonNode.Parse(await rotateResp.Content.ReadAsStringAsync())!;
var newKey = rotateJson["plainApiKey"]!.GetValue<string>();
```
</issue_to_address>Help me be more useful! Please click π or π on each comment and I'll use the feedback to improve your reviews.
|
Infisical secrets check: β No secrets leaked! π» Scan logs2026-05-07T15:38:47Z INF scanning for exposed secrets...
3:38PM INF 19 commits scanned.
2026-05-07T15:38:47Z INF scan completed in 68.6ms
2026-05-07T15:38:47Z INF no leaks found
|
|


π Description
Introduce Docker setup with a Dockerfile for building the Grimoire API service and a docker-compose.yml for local development. This setup improves the development environment's consistency and ease of use.
Update project files and dependencies to .NET 10, ensuring latest features and performance enhancements. Added .dockerignore to optimize Docker builds by excluding unneeded files.
Enhance testing coverage with the addition of end-to-end (E2E), integration, and unit tests using xUnit. These tests verify application functionality, reducing bugs and ensuring robust code changes. Added infrastructure for Testcontainers for isolated DB testing environments.
Modified SecretRepository to handle EF Core 10's limitations with nullable timestamp comparisons in SQL, improving query reliability for determining active secret versions.
β Checks
β’οΈ Does this introduce a breaking change?
Summary by Sourcery
Add containerization and health-check support for the API and significantly expand automated test coverage, including integration and E2E flows, while improving secret version querying compatibility with EF Core 10.
New Features:
Bug Fixes:
Enhancements:
Build:
Tests: